Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding dedicated import-export doc #668

Merged
merged 11 commits into from
Mar 1, 2021
Merged

Adding dedicated import-export doc #668

merged 11 commits into from
Mar 1, 2021

Conversation

jmwright
Copy link
Member

The skeleton of a new import-export doc to help users know what formats available and how to use the API.

@codecov
Copy link

codecov bot commented Feb 25, 2021

Codecov Report

Merging #668 (45227c9) into master (d350c1b) will increase coverage by 0.22%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #668      +/-   ##
==========================================
+ Coverage   94.03%   94.25%   +0.22%     
==========================================
  Files          31       31              
  Lines        6636     7207     +571     
  Branches      726      855     +129     
==========================================
+ Hits         6240     6793     +553     
- Misses        258      279      +21     
+ Partials      138      135       -3     
Impacted Files Coverage Δ
cadquery/occ_impl/shapes.py 93.43% <100.00%> (+1.53%) ⬆️
cadquery/occ_impl/assembly.py 99.10% <0.00%> (+4.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d350c1b...45227c9. Read the comment docs.

@jmwright jmwright marked this pull request as ready for review February 26, 2021 19:16
@jmwright
Copy link
Member Author

@adam-urbanczyk and @marcus7070 I think this is ready for review. Please pay special attention to the tolerance and angularTolerance parameter descriptions. I'm not sure that I grasp the nuances of each of those well enough to describe them properly.

@jmwright jmwright changed the title [WIP] Adding dedicated import-export doc Adding dedicated import-export doc Feb 27, 2021
doc/importexport.rst Outdated Show resolved Hide resolved
doc/importexport.rst Outdated Show resolved Hide resolved
doc/importexport.rst Outdated Show resolved Hide resolved
doc/importexport.rst Outdated Show resolved Hide resolved
@marcus7070
Copy link
Member

The last two commits were me (incompetently) trying to make git diff display the SVGs as "new file", instead of printing out a complete text diff for a file we'll never edit by hand.

jmwright and others added 4 commits February 27, 2021 07:22
Pull importDXF docstring into docs

Co-authored-by: Marcus Boyd <[email protected]>
Clarification of operation type called after toPending()

Co-authored-by: Marcus Boyd <[email protected]>
Updated description on TJS format

Co-authored-by: Marcus Boyd <[email protected]>
@jmwright
Copy link
Member Author

@adam-urbanczyk The tolerance default for exporters.export() is 0.1, but the default for tolerance in exportStl() is 1e-3. It seems like export() is always going to override the default for exportStl(), which seems like it might not be intended. I wanted to bring it up to confirm.

@adam-urbanczyk
Copy link
Member

Looks good, but you might want to state that angularTolerance is a maximum tangent error between the mesh and the original shape. I need to look into the tolerance value, that does not block the PR, right?

https://dev.opencascade.org/doc/overview/html/occt_user_guides__mesh.html

@jmwright
Copy link
Member Author

is a maximum tangent error between the mesh and the original shape.

Is that in addition to, or instead of, what's in the doc now?

@adam-urbanczyk
Copy link
Member

@jmwright after reading the docs again I conclude that I might've misinterpreted what is stated (and drawn) there. +1 for merging as-is.

@jmwright jmwright merged commit 0f32de9 into master Mar 1, 2021
@marcus7070
Copy link
Member

Thanks @jmwright!

@adam-urbanczyk
Copy link
Member

Thanks indeed, it'll be very helpful for the users!

@marcus7070 marcus7070 mentioned this pull request Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants